Are you sure it's a good idea? It gets an exclusive lock on the variable for potentially long period of time, restricting others from accessing it. Probably I would do something like fork/commit pattern, where at any time you can write the next value to the cache (aka fork), which will later be used by the consumer (block producer) when the time is right.

Something like (pseudocode):

enum TimeModeChange {
  SetTimestamp(u64),
  SetInterval(u64),
}

struct TimeManager {
    fork: RwLock<Vec<TimeModeChange>>, 
    state: RwLock<TimeManagerInner>
}

impl AdvanceTime for TimeManager {
    fn advance(&mut self) -> u64 {
        // consume all the actions from the queue
        // return the actual timestamp after calculations
    }
}

This way ReadTime will only have access to the "real" time available to the block producer, and the changes will be respected only after they are processed, but there is no single lock on the time manager state at any time, it's always short-living and we don't need to publicly expose it. I believe it's also not prone to TOCTOU since only the block producer is able to advance the state.

Agree that fork/commit would be helpful here. Especially to make some of the mutating calls non-blocking.

The way I see it, however, there is a problem with the rest of the proposal.

I believe it's also not prone to TOCTOU since only the block producer is able to advance the state.

Assuming you mean that BlockProducer is the only place that can advance state, then no that's not true. There are multiple entry points into InMemoryNode::seal_block as some endpoints require us to do synchronous block production.

Consider a scenario where there are two concurrent calls to anvil_mine: one with num=10,interval=100s, another with num=10,interval=1000s. Before this PR they would execute the following piece of code (simplified version from here):

for i in 0..num_blocks {
    if i != 0 {
        self.time.increase_time(interval);
    }
    self.seal_block([]);
}

There is an obvious problem here that each call's resulting blocks might not have exactly interval time between them. This PR is an attempt to ensure that we apply offsets specifically for the next N blocks we are about to produce and nothing else.

But even if we are okay with intervals bleeding into one another, there is a bigger problem inside seal_block (again simplified version of this):

let inner = self.read_guard();
// uses `TimestampManager` to predict the next timestamp
let system_env = inner.create_system_env();
drop(inner);
...
let inner = self.write_guard();
// takes next timestamp from `TimestampManager` and advances it
let expected_timestamp = inner.time.next_timestamp();
assert(block.timestamp.as_u64() == expected_timestamp);

There is a TOCTOU here as a competing block production thread could have already consumed the timestamp you were "supposed" to own. Now, IMHO this is mostly InMemoryNode's fault for being designed in such a way (the same exact problem also affects block/batch numbers and potentially even more things). I gave refactoring this a shot and this is a huge endeavour that I don't think I can spare time for right this moment.

To sum everything up, time essentially acts as an exclusive lock for block production which solves the problems I am trying to solve in this PR. I agree that it comes with heavy cost, so thinking about this further I might not need a write lock for the entire duration. What I actually need is a read lock that can be atomically upgraded to a write lock in short bursts (assuming I can do a successful minor refactoring of seal_block). I am not familiar with this pattern though so will have to look it up. It is also totally possible I misunderstood your proposal, so please elaborate if so.